CKS: handle VPC tier with no attached network ACL - #13961
Open
goal86sg wants to merge 1 commit into
Open
Conversation
A VPC tier without an attached ACL is a valid, supported state (aclid is optional on createNetwork; CloudStack stopped assigning a default-deny ACL unconditionally in CLOUDSTACK-2809). However, four CKS lifecycle sites compared the nullable Long returned by Network.getNetworkACLId() against the primitive long constants NetworkACL.DEFAULT_ALLOW / DEFAULT_DENY, auto-unboxing it and throwing NullPointerException when the tier had no ACL attached. This broke CKS cluster create/start/delete on a legitimate VPC tier configuration and left clusters stuck in Starting. Make the four comparisons null-safe with Objects.equals so that: - validateVpcTier accepts a null ACL (a valid state) instead of NPE-ing; - createVpcTierAclRules and setupKubernetesEtcdNetworkRules reach the existing NetworkACLService auto-create path (NetworkACLServiceImpl.createAclListIfNeeded) that creates and attaches a custom ACL when a rule is added with networkid and no aclid; - removeVpcTierAclRules treats a missing ACL as a no-op on delete. Adds regression unit tests: - KubernetesClusterManagerImplTest#testValidateVpcTierNullAclId - KubernetesClusterResourceModifierActionWorkerTest#removeVpcTierAclRulesNullAclIdIsNoOp Fixes apache#13761 Signed-off-by: Desmond <60381871+goal86sg@users.noreply.github.com>
goal86sg
force-pushed
the
cks-null-acl-fix-13761
branch
from
August 24, 2026 15:06
727a784 to
9daa97e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #13761
A VPC tier without an attached network ACL is a valid, supported state (
aclidis optional oncreateNetwork; CloudStack stopped assigning a default-deny ACL unconditionally in CLOUDSTACK-2809). However, four CKS lifecycle sites compared the nullableLongreturned byNetwork.getNetworkACLId()against the primitivelongconstantsNetworkACL.DEFAULT_ALLOW/NetworkACL.DEFAULT_DENY, auto-unboxing it and throwingNullPointerExceptionwhen the tier had no ACL attached. This broke CKS cluster create/start/delete on a legitimate VPC tier configuration and left clusters stuck inStarting.Root cause
NetworkACL.DEFAULT_ALLOW(=2) andNetworkACL.DEFAULT_DENY(=1) are primitivelongconstants (api/.../vpc/NetworkACL.java). So an expression likenetwork.getNetworkACLId() == NetworkACL.DEFAULT_ALLOWauto-unboxes the nullableLongand throwsNullPointerException: Cannot invoke "java.lang.Long.longValue()"when the tier has no ACL attached.Fix
Make the four comparisons null-safe with
Objects.equals(value comparison, no unboxing):KubernetesClusterManagerImplvalidateVpcTier== DEFAULT_DENYObjects.equals(..., DEFAULT_DENY)— null is a valid state, not rejectedKubernetesClusterResourceModifierActionWorkercreateVpcTierAclRules== DEFAULT_ALLOW(early return)Objects.equals(..., DEFAULT_ALLOW)— null falls through to provisioningKubernetesClusterResourceModifierActionWorkerremoveVpcTierAclRules== DEFAULT_ALLOW(early return)null || Objects.equals(..., DEFAULT_ALLOW)— no ACL ⇒ no-op on deleteKubernetesClusterStartWorkersetupKubernetesEtcdNetworkRules!= DEFAULT_ALLOW!Objects.equals(..., DEFAULT_ALLOW)— null falls through to provisioningThis lets CKS reach the existing
NetworkACLServiceauto-create path (NetworkACLServiceImpl.createAclListIfNeeded), which creates and attaches a custom ACL when a rule is added withnetworkidand noaclid— exactly the behavior the issue expects. The downstream paths are already null-safe (NetworkACLItemDaoImpl.listByACL(null)returns an empty list), so only the four comparisons needed changing.Types of changes
How Has This Been Tested?
mainbefore this change and passing after:KubernetesClusterManagerImplTest#testValidateVpcTierNullAclId— null ACL is accepted (no NPE / no rejection).KubernetesClusterResourceModifierActionWorkerTest#removeVpcTierAclRulesNullAclIdIsNoOp— delete with no ACL is a no-op (no NPE).kubernetes-serviceplugin unit-test module locally (JDK 17): all tests pass —KubernetesClusterManagerImplTest48/48 andKubernetesClusterResourceModifierActionWorkerTest8/8 — including the two new regression tests.createVpcTierAclRules,setupKubernetesEtcdNetworkRules) use the identical null-safeObjects.equalspattern and are exercised by the reporter's regression suite referenced in CKS lifecycle fails on VPC tier without an attached network ACL #13761; the existing CKS Marvin/integration tests cover the broader create/start/delete lifecycle.Checklist